IO-747 Make commons-io a proper JPMS module#268
Conversation
|
This PR is similar to apache/commons-fileupload#107 |
| <profile> | ||
| <id>release</id> | ||
| <properties> | ||
| <!-- Enable Moditect for releases. This requires JDK 9+ for the build --> |
There was a problem hiding this comment.
That's not good. I build releases with Java 8 since we want to bind to the JRE 8 APIs and not 9 especially with problems due to the NIO Buffer class changes. I think we have other Commons components that are OSGi and JPMS modules, so you might want to look around. Why is there not a module-info.java somewhere?
There was a problem hiding this comment.
What are the issues with NIO Buffer classes if you build with Java 9 and --release=8 ?
There was a problem hiding this comment.
@garydgregory I think, that release building with Java 9+ is a more than reasonable requirement, if we want to fit into the Java 9 ecosphere. (There is no question, that we want that, isn't there?)
@martin-g I'd like to see two improvements:
- Move the plugin definition into a profile, that enables the moditect-maven-plugin only when building with Java 9+. Alternatively, set the property moditect.skip for building with Java 8.
- Make sure, that the apache-release profile enforces building with Java 9+.
Regarding commons-parent: My personal stance is, that we have covered commons-io, and commons-fileupload now (Perhaps, cover a few others, in case of demand.) Let's wait, and see, until we did a release with that, then approach parent.
|
Thank you @martin-g for raising this! We also need JPMS-zed version of commons-io and commons-collections4. |
|
Why does this patch not follow the recommeded pattern from https://maven.apache.org/plugins/maven-compiler-plugin/examples/module-info.html ? |
|
I used Moditect because it generates the module-info.java. With maven-compiler-plugin module-info.java must be in I don't mind closing this PR if someone provides different way to achieve the end goal! |
Enforce that Java 9+ is being used for releases Suggested-by: Jochen Wiedmann at https://github.com/apache/commons-io/pull/268/files#r707160987
|
One minor nitpick: As I understand it, there is now a hardcoded version 8 for the Java compiler. IIRC, there is a property in commons-parent for that. Please, change that to use that property. Thanks! |
|
@jochenw I guess you mean https://github.com/apache/commons-parent/blob/dd3e4bc64628c0e1ae15cf16a48a04bdfd8dbcc9/pom.xml#L109-L110 Those are overwritten here - Lines 297 to 298 in 319a540 But there is no such for |
If so, you can override the property maven.compiler.release with the value "8". Should work fine, as far as I know. |
|
FYI: This is how we use toolchains over at Log4j, see the |
@jochenw I don't follow you! There is no mention of |
No description provided.